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

Dynamically adjust recent template text based on window size #2011

Merged
merged 4 commits into from
Apr 4, 2019

Conversation

zfengms
Copy link
Contributor

@zfengms zfengms commented Apr 1, 2019

fix #1969 Dynamically adjust recent template text based on window size

}));
}

@HostListener("window:resize", ["$event"])
Copy link
Member

Choose a reason for hiding this comment

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

you should be able to do all of that just with css. This looks a bit too complex for what it should be

Copy link
Member

Choose a reason for hiding this comment

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

The issue with CSS is that it trims the end of the string, we want the start of the string to be trimmed so we can still see the template name, if we can do this with CSS then go for it. Just not sure it's doable.

Copy link
Member

Choose a reason for hiding this comment

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

First yes it is possible to ellipsis at the beginning, we talked with zhiqiang the issue here is the title is not just the filename it contains the entire title shown on the submit form Run template foo.ts. Adding ... in the middle depending on what the text start with is quite hacky and will break at some point(Localization for example) For now we'll just make sure the button doesn't show up alone on the new line. Then we can think of a way to change so that only the path(Or action name) is passed to the recent template so that can be stripped

@codecov
Copy link

codecov bot commented Apr 1, 2019

Codecov Report

Merging #2011 into stable will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           stable   #2011   +/-   ##
======================================
  Coverage      65%     65%           
======================================
  Files        1014    1014           
  Lines       27718   27718           
  Branches     4941    4941           
======================================
  Hits        18018   18018           
  Misses       9684    9684           
  Partials       16      16

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa9f4f9...5c1fee1. Read the comment docs.

Copy link
Member

@timotheeguerin timotheeguerin left a comment

Choose a reason for hiding this comment

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

Just update the changelog.md as well and this is good to go

@zfengms zfengms merged commit 3559c65 into stable Apr 4, 2019
@zfengms zfengms deleted the bugfix/recenttemplatelayout branch April 4, 2019 20: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.

4 participants