Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move version chip to footer #3518

Merged
merged 11 commits into from
Oct 16, 2024
Merged

Conversation

ehab-hassan
Copy link
Contributor

@ehab-hassan ehab-hassan commented Oct 9, 2024

Description

the version chip overlaps with the sidebar extending so ll move it to footer

Changes

Move version chip to footer

image

Related Issues

#3514

Tested Scenarios

A list of scenarios tried to match the deliverables

Documentation PR

For UI changes, Please provide the Documetation PR on info_grid

To consider

Preliminary Checks:

  • Does it completely address the issue linked?
  • What about edge cases?
  • Does it meet the specified acceptance criteria?
  • Are there any unintended side effects?
  • Does the PR adhere to the team's coding conventions, style guides, and best practices?
  • Does it integrate well with existing features?
  • Does it impact the overall performance of the application?
  • Are there any bottlenecks or slowdowns?
  • Has it been optimized for efficiency?
  • Has it been adequately tested with unit, integration, and end-to-end tests?
  • Are there any known defects or issues?
  • Is the code well-documented?
  • Are changes to documentation reflected in the code?

UI Checks:

  • If a UI design is provided/ does it follow it?
  • Does every button work?
  • Is the data displayed logical? Is it what you expected?
  • Does every validation work?
  • Does this interface feel intuitive?
  • What about slow network? Offline?
  • What if the gridproxy/graphql/chain is failing?
  • What would a first time user have a hard time navigating here?

Code Quality Checks:

  • Code formatted/linted? Are there unused imports? .. etc
  • Is there redundant/repeated code?
  • Are there conditionals that are always true or always false?
  • Can we write this more concisely?
  • Can we reuse this code? If yes, where?
  • Will the changes be easy to maintain and update in the future?
  • Can this code become too complex to understand for other devs?
  • Can this code cause future integration problems?

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings
  • Screenshots/Video attached (needed for UI changes)

@ehab-hassan ehab-hassan marked this pull request as ready for review October 9, 2024 12:11
@samaradel
Copy link
Contributor

Please hide the | and the version if it's not existed
image

<div class="mb-12 d-flex justify-center text-subtitle-2">
<p>
&#169; {{ new Date().getFullYear() }} ThreeFoldTech
<span v-if="version !== 'No version to show'">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be a good idea to place 'No version to show' in a constant @AhmedHanafy725

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea is to show the version to the user, if there is no version, there is no need to tell him that
@AhmedHanafy725 what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be a good idea to place 'No version to show' in a constant @AhmedHanafy725

Done @amiraabouhadid can you check please

@amiraabouhadid
Copy link
Contributor

@ehab-hassan hey I actually meant declaring 'no current version to show' as an app wide constant as it is used in multiple places, not as a message string in a single component

@ehab-hassan ehab-hassan merged commit fccc6af into development Oct 16, 2024
10 checks passed
@ehab-hassan ehab-hassan deleted the development_version_chip_UI branch October 16, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants